Skip to content

Graceful SP consume error handling - #3

Open
nerdyworm wants to merge 1 commit into
mainfrom
graceful-consume-errors
Open

Graceful SP consume error handling#3
nerdyworm wants to merge 1 commit into
mainfrom
graceful-consume-errors

Conversation

@nerdyworm

Copy link
Copy Markdown

Why

SP consume failures (invalid_relay_state etc.) returned a raw access_denied 403 — a dead end for users whose session cookie didn't round-trip between signin and the IdP's consume POST (replayed/refreshed response, second login tab, dropped cookie in Citrix-style browser contexts). Malformed SAMLResponses crashed to a 500 because esaml raises. Surfaced by Houston Methodist users; consumed by blockit/api#6660.

What

  • Session-loss failures (invalid_relay_state / invalid_idp_id / invalid_target_url) 302 back to a fresh signin with ?samly_retry=1. The IdP usually still holds a live SSO session, so the retry completes without user interaction.
  • Loop guard: the retried signin prefixes its generated RelayState with retry_. RelayState is echoed back by the IdP, so the guard holds even when the session cookie is the thing that's broken — a second failure cannot redirect-loop.
  • Repeat or non-recoverable failures render a styled error page with a "Try signing in again" link and the reason code, instead of raw 403 text.
  • Malformed SAMLResponses are rescued to {:error, :malformed_response} and get the same page instead of a 500.

IdP-initiated flow and all validation logic are unchanged.

Testing

Covered by apps/blockit_web/test/blockit_web/sso_consume_error_test.exs in blockit/api#6660: retry redirect, loop guard, error page, malformed-response handling through the real endpoint, and the marker riding through the signin auto-submit form.

🤖 Generated with Claude Code

On consume failure, auto-retry the signin once for session-loss shaped
errors (invalid relay state / idp id / target url). The retry marker is
carried in the RelayState, which the IdP echoes back, so the loop guard
holds even when the session cookie is what's broken.

Repeat or non-recoverable failures render a human error page with a
try-again link instead of a raw access_denied 403. Malformed
SAMLResponses (esaml raises) now also get the error page instead of a
500.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant